Close process streams after exit to work around JDK-4311711 hang - #412
Close process streams after exit to work around JDK-4311711 hang#412elharo wants to merge 1 commit into
Conversation
ascheman
left a comment
There was a problem hiding this comment.
Thanks for tackling this @elharo — #270 is a real hang and the instinct (force EOF by closing the stream) is right for the stuck case. But I ran an empirical check against this branch vs master, and the unconditional close introduces a spurious-failure regression on the normal path.
Setup: seq 1 5000 (exits 0, 5000 lines) via executeCommandLine, 60 runs, counting non-zero exits, truncated output, and thrown CommandLineException. Same probe on master and on this branch, JDK 17 and JDK 25.0.3:
| branch | JDK 17 | JDK 25.0.3 |
|---|---|---|
master |
0/60 exceptions | 0/60 |
| this PR | 6–7/60 (~10%) | 7/60 (~12%) |
All failures are CommandLineException: Failure processing stdout on a command that exited 0 with complete output (no truncation observed, so it's not data loss — it's a false failure).
Why: after waitFor() returns, the pumper threads are often still draining buffered output. Closing getInputStream()/getErrorStream() out from under an in-flight readLine() makes it throw IOException, which StreamPumper.run() stores in exception, and call() then rethrows as CommandLineException. A separate probe confirms that when the pumper is idle-blocked on an empty stream (the actual JDK-4311711 hang), the close yields a clean EOF with no exception — so the close only misfires when there's in-flight data.
Suggested direction: make the close a fallback, not the default — waitUntilDone(timeout) first (pumpers reach EOF on their own in the common case, as master shows), and only force-close if they're still stuck past the timeout. Additionally, mark the pumper as intentionally-closed (there's already a disable() hook) so an IOException arising from our close is treated as EOF rather than recorded as a stdout/stderr failure. That keeps the hang fix while removing the false failures.
Happy to contribute the probe as a proper regression test if useful — it reliably reproduces the ~10% false-failure on this branch and stays green on master.
After
Process.waitFor()returns, close the process's stdout, stderr, and stdin streams. This works around JDK-4311711 whereProcess.getInputStream().read()can hang indefinitely even after the process has terminated.Without this fix,
CommandLineUtils.executeCommandLineAsCallable()can hang forever on some JVMs because theStreamPumperthreads are blocked inBufferedReader.readLine()on the process's output streams, which never reach EOF.Fixes #270